Skip to content

Enable Ycheck after labelDef. Fixes #701 #724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jul 22, 2015
Merged

Enable Ycheck after labelDef. Fixes #701 #724

merged 5 commits into from
Jul 22, 2015

Conversation

DarkDimius
Copy link
Contributor

LabelDefs reorders labels. As a result of reordering label-def defined inside other label-def could be lifted outside.

Those commits make TreeChecker aware of this.

@DarkDimius
Relax Ycheck for labels after LabelDefs
61c30f8 
LabelDefs reorders labels. As a result of reordering
label-def defined inside other label-def could be lifted outside.
LabelDefs doesn't update owner chains to represent this.

Making treeChecker aware of this.
@DarkDimius
Enable Ycheck after labelDef. Fixes scala#701
6369d6f
@DarkDimius
Copy link
Contributor Author

/rebuild

1 similar comment
@adriaanm
Copy link
Contributor

/rebuild

@DarkDimius
Copy link
Contributor Author

@odersky please review

@DarkDimius
Copy link
Contributor Author

On my machine, time that it takes for the full junit testing suite to pass went from 4 min to 4 min 40 secs.
I would actually prefer to enable -Ycheck:all, at least in the Jenkins testing suite, but frontend currently does not pass Ycheck #725.

@odersky
Copy link
Contributor

odersky commented Jul 16, 2015

I think 40 secs is sufficiently bad not to do it. Compromise proposal:
Let's test some files with -Ycheck:labeldefs and others with
-Ycheck:restoreScopes. Maybe pos with labelchecks and run with
restoreScopes (or vice versa)?

On Thu, Jul 16, 2015 at 10:50 AM, Dmitry Petrashko <[email protected]

wrote:

On my machine, time that it takes for the full junit testing suite to pass
went from 4 min to 4 min 40 secs.
I would actually prefer to enable -Ycheck:all, at least in the Jenkins
testing suite, but frontend currently does not pass Ycheck #725
#725.

—
Reply to this email directly or view it on GitHub
#724 (comment).

Martin Odersky
EPFL

@DarkDimius
Copy link
Contributor Author

During the meeting we agreed to leave -Ycheck:labeldefs disabled in test suite that is ran locally, but enable -Ycheck:all in Jenkins CI.

@odersky
Copy link
Contributor

odersky commented Jul 20, 2015

@DarkDimius What needs to be done scriptwise to achieve this?

@DarkDimius
Remove .travis.yml
fa3265e 
We no longer use Travis, and testing infrastructure is not guaranteed to work there.
@DarkDimius
Enable -Ycheck:all in Jenkins.
8d26619 
Done by setting an environment variable and checking it in runtime.
This enables Ycheck:all for all kinds of tests, including partest.
@DarkDimius
Disable Ycheck:all until scala#725 is fixed.
7eef035
@DarkDimius
Copy link
Contributor Author

@odersky, scrips are updated. Ycheck:all is not enabled due to #725

DarkDimius added a commit that referenced this pull request Jul 22, 2015
@DarkDimius
Merge pull request #724 from dotty-staging/labels-Ycheck
b3ca8e1 
Enable Ycheck after labelDef. Fixes #701
@DarkDimius DarkDimius merged commit b3ca8e1 into scala:master Jul 22, 2015
@allanrenucci allanrenucci deleted the labels-Ycheck branch December 14, 2017 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DarkDimius @adriaanm @odersky